store TiDB server info to PD and add http api handle #7082

crazycs520 · 2018-07-18T02:07:43Z

What have you changed? (mandatory)

This PR is for issue#6961

Store TiDB server static information to PD when DDL server start up and delete when server down. The information will not update when TiDB is running.So I call this static information.
Add 2 http-api.
- {TiDBIP}:10080/info request for TiDBIP server information.
- {TiDBIP}:10080/info/all requSchemaSyncerest for all TiDB server information.
static information

ID         string `json:"ddl_id"`
IP         string `json:"ip"`
Port       uint   `json:"listening_port"`
StatusPort uint   `json:"status_port"`
Lease      string `json:"lease"`
Version string `json:"version"`
GitHash string `json:"git_hash"`

What is the type of the changes? (mandatory)

New feature (non-breaking change which adds functionality)

How has this PR been tested? (mandatory)

add in tidb-test. https://github.com/pingcap/tidb-test/pull/593

Does this PR affect documentation (docs/docs-cn) update? (mandatory)

Yes

Does this PR affect tidb-ansible update? (mandatory)

Yes

Does this PR need to be added to the release notes? (mandatory)

Store TiDB information to PD and add http-api to get TiDB and cluster information.

Refer to a related PR or issue link (optional)

Benchmark result if necessary (optional)

Add a few positive/negative examples (optional)

winkyao · 2018-07-18T02:10:39Z

Does this PR need to be added to the release notes? (mandatory) YES

…ver_info_to_pd

ciscoxll · 2018-07-18T09:17:41Z

ddl/ddl.go

@@ -510,6 +525,46 @@ func (d *ddl) SetBinlogClient(binlogCli interface{}) {
 	d.binlogCli = binlogCli
 }

+func (d *ddl) GetServerInfo() *util.DDLServerInfo {


You could write it like this.

func (d *ddl) GetServerInfo() *util.DDLServerInfo { cfg := config.GetGlobalConfig() info := &util.DDLServerInfo{ ID: d.uuid, IP: cfg.AdvertiseAddress, StatusPort: cfg.Status.StatusPort, Lease: cfg.Lease, } info.Version = mysql.ServerVersion info.GitHash = printer.TiDBGitHash return info }

…ver_info_to_pd

crazycs520 · 2018-07-19T10:51:35Z

@winkyao @ciscoxll @zimulala PTAL

shenli · 2018-07-20T02:57:46Z

ddl/ddl.go

@@ -207,6 +209,15 @@ type DDL interface {

 	// SetBinlogClient sets the binlog client for DDL worker. It's exported for testing.
 	SetBinlogClient(interface{})
+
+	// GetServerInfo get self DDL server static information.
+	GetServerInfo() *util.DDLServerInfo


This is not related with DDL.

XuHuaiyu

Documentation should be updated?

XuHuaiyu · 2018-07-20T07:34:38Z

ddl/ddl.go

@@ -376,6 +387,10 @@ func (d *ddl) close() {
 	if err != nil {
 		log.Errorf("[ddl] remove self version path failed %v", err)
 	}
+	err = d.schemaSyncer.RemoveSelfServerInfo()
+	if err != nil {
+		log.Errorf("[ddl] remove self server info path failed %v", err)


remove path?

XuHuaiyu · 2018-07-20T07:35:30Z

ddl/syncer.go

@@ -20,11 +20,14 @@ import (
 	"sync"
 	"time"

+	"encoding/json"


put this line to line 17

XuHuaiyu · 2018-07-20T07:49:22Z

ddl/syncer.go

+			time.Sleep(200 * time.Millisecond)
+			continue
+		}
+		if err == nil && len(resp.Kvs) > 0 {


no need to check err == nil here

XuHuaiyu · 2018-07-20T07:49:43Z

ddl/syncer.go

+			err := json.Unmarshal(resp.Kvs[0].Value, info)
+			if err != nil {
+				log.Infof("[syncer] get ddl server info, ddl %s json.Unmarshal %v failed %v.", resp.Kvs[0].Key, resp.Kvs[0].Value, err)
+				return nil, err


errors.Trace(err)

XuHuaiyu · 2018-07-20T07:50:50Z

ddl/syncer.go

+			err := json.Unmarshal(kv.Value, info)
+			if err != nil {
+				log.Infof("[syncer] get all ddl server info, ddl %s json.Unmarshal %v failed %v.", kv.Key, kv.Value, err)
+				return nil, err


errors.Trace(err)

XuHuaiyu · 2018-07-20T07:55:41Z

Maybe it's better to specify what static information does this PR store to PD.

XuHuaiyu · 2018-07-20T08:03:59Z

ddl/syncer.go

+	// GetAllServerInfoFromPD get all DDL servers information from PD.
+	GetAllServerInfoFromPD(ctx context.Context) (map[string]*util.DDLServerInfo, error)
+	// UpdateSelfServerInfo store DDL server information to PD.
+	UpdateSelfServerInfo(ctx context.Context, info *util.DDLServerInfo) error


Since the info will not be updated during running,
I think that name it as StoreSelfServerInfoToPD is enough

shenli · 2018-07-22T03:34:42Z

@crazycs520 Please address the comments.

zimulala · 2018-08-13T08:23:19Z

domain/info.go

+	}
+	info, ok := infoMap[id]
+	if !ok {
+		return nil, errors.Errorf("[infoSyncer] get %s failed", key)


I think "info-syncer" is better.

zimulala · 2018-08-13T08:25:13Z

domain/info.go

+	return errors.Trace(is.newSessionAndStoreServerInfo(ctx, owner.NewSessionDefaultRetryCnt))
+}
+
+//GetServerInfo gets self server static information.


s//Get// Get

zimulala · 2018-08-13T08:38:54Z

server/http_handler.go

+		log.Error(err)
+		return
+	}
+	ownerID, err := do.DDL().OwnerManager().GetOwnerID(ctx)


Do we need to add a timeout to ctx?

zimulala

LGTM

shenli · 2018-08-09T15:05:27Z

ddl/syncer.go

@@ -83,6 +83,8 @@ type SchemaSyncer interface {
 	// the latest schema version. If the result is false, wait for a while and check again util the processing time reach 2 * lease.
 	// It returns until all servers' versions are equal to the latest version or the ctx is done.
 	OwnerCheckAllVersions(ctx context.Context, latestVer int64) error
+	// GetSession return the session lease id of SchemaSyncer.
+	GetSessionLeaseID() clientv3.LeaseID


s/GetSessionLeaseID/GetLeaseID/

shenli · 2018-08-14T03:00:09Z

domain/info.go

+// InfoSessionTTL is the etcd session's TTL in seconds. It's exported for testing.
+var InfoSessionTTL = 10 * 60
+
+// InfoSyncer stores server info to etcd when when the tidb-server starts and delete when tidb-server shuts down.


when when -> when

shenli · 2018-08-14T03:01:12Z

domain/info.go

+}
+
+// ServerInfo is server static information.
+// It will not update when server running. So please only put static information in ServerInfo struct.


s/update/be updated/ or change
when server running -> during tidb-server process lifetime

shenli · 2018-08-14T03:04:43Z

domain/info.go

+		StatusPort: cfg.Status.StatusPort,
+		Lease:      cfg.Lease,
+	}
+	info.Version = mysql.ServerVersion


Why not move the two lines into the above block?

Version and GitHash belong to ServerVersionInfo. ServerInfo contain ServerVersionInfo with no attribute name.

ciscoxll · 2018-08-14T03:24:02Z

ddl/syncer.go

-func (s *schemaVersionSyncer) putKV(ctx context.Context, retryCnt int, key, val string,
+// PutKVToEtcd puts key value to etcd.
+// etcdCli is client of etcd.
+// retryCnt is retry time when When an error occurs.


s/ when when / when

shenli · 2018-08-14T06:54:52Z

domain/info.go

+)
+
+// InfoSessionTTL is the etcd session's TTL in seconds. It's exported for testing.
+var InfoSessionTTL = 10 * 60


1 mins is long enough.

shenli · 2018-08-14T06:55:53Z

domain/info.go

+
+// Restart restart the info syncer with new session leaseID and store server info to etcd again.
+func (is *InfoSyncer) Restart(ctx context.Context) error {
+	return errors.Trace(is.newSessionAndStoreServerInfo(ctx, owner.NewSessionRetryUnlimited))


Why use NewSessionRetryUnlimited?

I see in schemaVersionSyncer. Restart() use NewSessionRetryUnlimited when Restart, so...
How about your option?

crazycs520 · 2018-08-14T10:55:20Z

@shenli PTAL

…ver_info_to_pd

ciscoxll

LGTM

crazycs520 · 2018-08-14T11:26:19Z

/run-all-tests

shenli · 2018-08-15T00:56:33Z

/run-all-tests

…ver_info_to_pd

crazycs520 · 2018-08-15T02:23:18Z

/run-all-tests

* See pingcap/tidb#7082 for details of the API

* tidb: add info collected from TiDB's server API * See pingcap/tidb#7082 for details of the API * tidb: minor cleanup of pdctl code * tidb: check wheather tidb server api is supported * tidb: fix runtime error when collecting pdctl

crazycs520 added 4 commits July 18, 2018 00:18

add get all server info api

3918624

rename and add comment

2370bb0

add cluster info

b574aff

remove isOwner from server static info

aca6b97

winkyao added the release-note Denotes a PR that will be considered when it comes time to generate release notes. label Jul 18, 2018

refine comment

d5a1922

crazycs520 added the component/DDL-need-LGT3 label Jul 18, 2018

Merge branch 'master' of https://github.com/pingcap/tidb into put_ser…

3c8f418

…ver_info_to_pd

crazycs520 force-pushed the put_server_info_to_pd branch from b9f7388 to 1f44a8a Compare July 18, 2018 05:25

add lease to server info and change tidb_http_api.md

24eea21

crazycs520 force-pushed the put_server_info_to_pd branch from 1f44a8a to 24eea21 Compare July 18, 2018 05:30

crazycs520 added 2 commits July 18, 2018 13:39

refine log

f0007a4

refine http-api ddl/info

159a7b4

crazycs520 force-pushed the put_server_info_to_pd branch from 1fed1c2 to 159a7b4 Compare July 18, 2018 05:55

ciscoxll reviewed Jul 18, 2018

View reviewed changes

crazycs520 added 2 commits July 19, 2018 11:14

refine code

f143709

Merge branch 'master' of https://github.com/pingcap/tidb into put_ser…

256798e

…ver_info_to_pd

crazycs520 force-pushed the put_server_info_to_pd branch from 8e9854a to 256798e Compare July 19, 2018 10:49

rename variable

8bd6c3d

shenli reviewed Jul 20, 2018

View reviewed changes

XuHuaiyu reviewed Jul 20, 2018

View reviewed changes

crazycs520 added status/WIP and removed status/WIP labels Jul 22, 2018

refactor code

80cdd6a

zimulala reviewed Aug 13, 2018

View reviewed changes

address comment

6060b57

zimulala reviewed Aug 13, 2018

View reviewed changes

zimulala added status/LGT2 Indicates that a PR has LGTM 2. component/DDL-need-LGT3 and removed component/DDL-need-LGT3 status/LGT1 Indicates that a PR has LGTM 1. labels Aug 13, 2018

shenli reviewed Aug 14, 2018

View reviewed changes

address comment

2a97788

ciscoxll reviewed Aug 14, 2018

View reviewed changes

shenli reviewed Aug 14, 2018

View reviewed changes

crazycs520 added 2 commits August 14, 2018 15:09

address comment

2da0623

address comment

26c8b25

Merge branch 'master' of https://github.com/pingcap/tidb into put_ser…

93c76ce

…ver_info_to_pd

ciscoxll reviewed Aug 14, 2018

View reviewed changes

ciscoxll previously approved these changes Aug 14, 2018

View reviewed changes

crazycs520 added 2 commits August 15, 2018 09:21

Merge branch 'master' of https://github.com/pingcap/tidb into put_ser…

f8b7b98

…ver_info_to_pd

Merge branch 'master' of https://github.com/pingcap/tidb into put_ser…

9831a1f

…ver_info_to_pd

crazycs520 dismissed ciscoxll’s stale review via 9831a1f August 15, 2018 02:04

crazycs520 force-pushed the put_server_info_to_pd branch from 3e4fa22 to 9831a1f Compare August 15, 2018 02:04

ciscoxll approved these changes Aug 15, 2018

View reviewed changes

Merge branch 'master' into put_server_info_to_pd

34d12be

crazycs520 merged commit 9fc67b9 into pingcap:master Aug 15, 2018

AstroProfundis added a commit to AstroProfundis/tidb-insight that referenced this pull request Aug 20, 2018

tidb: add info collected from TiDB's server API

2d65c64

* See pingcap/tidb#7082 for details of the API

you06 added the sig/sql-infra SIG: SQL Infra label Mar 4, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

store TiDB server info to PD and add http api handle #7082

store TiDB server info to PD and add http api handle #7082

crazycs520 commented Jul 18, 2018 •

edited

Loading

winkyao commented Jul 18, 2018

ciscoxll Jul 18, 2018

crazycs520 Jul 19, 2018

crazycs520 commented Jul 19, 2018

shenli Jul 20, 2018

XuHuaiyu left a comment

XuHuaiyu Jul 20, 2018

XuHuaiyu Jul 20, 2018

XuHuaiyu Jul 20, 2018

XuHuaiyu Jul 20, 2018

XuHuaiyu Jul 20, 2018

XuHuaiyu commented Jul 20, 2018

XuHuaiyu Jul 20, 2018

shenli commented Jul 22, 2018

zimulala Aug 13, 2018

zimulala Aug 13, 2018

zimulala Aug 13, 2018

zimulala left a comment

shenli Aug 9, 2018

shenli Aug 14, 2018

shenli Aug 14, 2018

shenli Aug 14, 2018

crazycs520 Aug 14, 2018

ciscoxll Aug 14, 2018

crazycs520 Aug 14, 2018

shenli Aug 14, 2018

shenli Aug 14, 2018

crazycs520 Aug 14, 2018

crazycs520 commented Aug 14, 2018

ciscoxll left a comment

crazycs520 commented Aug 14, 2018

shenli commented Aug 15, 2018

crazycs520 commented Aug 15, 2018

store TiDB server info to PD and add http api handle #7082

store TiDB server info to PD and add http api handle #7082

Conversation

crazycs520 commented Jul 18, 2018 • edited Loading

What have you changed? (mandatory)

What is the type of the changes? (mandatory)

How has this PR been tested? (mandatory)

Does this PR affect documentation (docs/docs-cn) update? (mandatory)

Does this PR affect tidb-ansible update? (mandatory)

Does this PR need to be added to the release notes? (mandatory)

Refer to a related PR or issue link (optional)

Benchmark result if necessary (optional)

Add a few positive/negative examples (optional)

winkyao commented Jul 18, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

crazycs520 commented Jul 19, 2018

Choose a reason for hiding this comment

XuHuaiyu left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

XuHuaiyu commented Jul 20, 2018

Choose a reason for hiding this comment

shenli commented Jul 22, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

zimulala left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

crazycs520 commented Aug 14, 2018

ciscoxll left a comment

Choose a reason for hiding this comment

crazycs520 commented Aug 14, 2018

shenli commented Aug 15, 2018

crazycs520 commented Aug 15, 2018

crazycs520 commented Jul 18, 2018 •

edited

Loading